[Snippets] Add debug caps for dumping snippets parameters #28378

aobolensk · 2025-01-10T15:06:17Z

Details:

Add infrastructure to dump snippets parameters using OV_SNIPPETS_DUMP_PARAMS environment variable
Enable caps for brgemm snippet

Tickets:

137302

src/common/snippets/docs/debug_capabilities/parameters_dump.md

src/common/snippets/include/snippets/lowered/pass/brgemm_blocking.hpp

src/common/snippets/include/snippets/utils/params_dumper.hpp

src/common/snippets/docs/debug_capabilities/parameters_dump.md

src/common/snippets/include/snippets/utils/debug_caps_config.hpp

src/common/snippets/include/snippets/lowered/pass/brgemm_blocking.hpp

v-Golubev

It can be also useful to dump Subgraph's name as well. Let's discuss possible options offline

src/common/snippets/include/snippets/lowered/pass/brgemm_debug_params.hpp

src/plugins/intel_cpu/src/nodes/subgraph.cpp

aobolensk · 2025-01-15T14:13:58Z

It can be also useful to dump Subgraph's name as well. Let's discuss possible options offline

Done

src/common/snippets/include/snippets/lowered/pass/brgemm_debug_params.hpp

src/plugins/intel_cpu/src/nodes/subgraph.cpp

src/common/snippets/include/snippets/lowered/pass/brgemm_debug_params.hpp

src/common/snippets/docs/debug_capabilities/parameters_dump.md

v-Golubev

LGTM 👍

src/common/snippets/include/snippets/lowered/pass/brgemm_debug_params.hpp

IvanNovoselov · 2025-01-30T09:24:49Z

src/common/snippets/src/op/perf_count.cpp

+    : PerfCountEndBase({pc_begin}),
+      accumulation(0ul),
+      iteration(0u),
+      dumpers(dumpers) {


Please avoid redundant copy, since you are passing by value

Suggested change

dumpers(dumpers) {

dumpers(std::move(dumpers)) {

IvanNovoselov · 2025-01-30T09:29:42Z

src/common/snippets/include/snippets/op/perf_count.hpp

+class Dumper {
+public:
+    Dumper() = default;
+    Dumper(const Dumper&) = delete;


Interesting 🤔
Why do we delete default copy constructors here and in the derived classes?
We don't manage any resources using pointers, so the default-generated would work as expected.

Debug leftovers, removed

IvanNovoselov · 2025-01-30T09:38:54Z

src/common/snippets/src/op/perf_count.cpp

+    auto accumulation = node->get_accumulation();
+    auto iteration = node->get_iteration();
+    OPENVINO_ASSERT(accumulation.size() == iteration.size(), "accumulation size should be the same as iteration size in perf_count_end node.");
+    auto iterator_iter = iteration.begin();
+    auto iterator_acc = accumulation.begin();
+    int t_num = 0;
+    uint64_t avg_max = 0;
+    std::cout << "Perf count data in perfCountEnd node with name " << node->get_friendly_name() << " is:" << std::endl;
+    for (; iterator_iter != iteration.end(); ++iterator_iter, ++iterator_acc) {
+        const auto iter = *iterator_iter;
+        const auto acc = *iterator_acc;
+        uint64_t avg = iter == 0 ? 0 : acc / iter;
+        if (avg > avg_max)
+            avg_max = avg;
+        std::cout << "accumulated time:" << acc << "ns, iteration:" << iter << " avg time:" << avg << "ns" << " on thread:" << t_num << std::endl;
+        t_num++;
+    }
+
+    // max time of all threads: combine for reduce max
+    auto BinaryFunc = [](const uint64_t& a, const uint64_t& b) {
+        return a >= b ? a : b;
+    };
+
+    // max accumulation
+    uint64_t acc_max = accumulation.combine(BinaryFunc);
+    std::cout << "max accumulated time:" << acc_max << "ns" << std::endl;
+    // max avg
+    std::cout << "max avg time:" << avg_max << "ns" << std::endl;


I see that you decided to retain the old behavior, where the output is printed on every iteration instead of printing it once in the destructor. Is there any particular reason why?
As we discussed on the meeting, this approach have higher runtime overheads that might be important when we have nested perf counters. Moreover now we have two dumpers with mismatching behavior, which might be confusing.

No reason, actually. Moved to destructor, aligned behavior

IvanNovoselov · 2025-02-04T17:25:08Z

src/common/snippets/src/op/perf_count.cpp

+CSVDumper::CSVDumper(const std::string& csv_path) : csv_path(csv_path) {}
+
+CSVDumper::~CSVDumper() {
+    dump_brgemm_params_to_csv();


It looks like the dump_brgemm_params_to_csv method is redundant, please remove.

Agree, removed

IvanNovoselov · 2025-02-04T17:30:30Z

src/common/snippets/include/snippets/op/perf_count.hpp

+class ConsoleDumper : public Dumper {
+public:
+    ConsoleDumper() = default;
+    ConsoleDumper(const ConsoleDumper&) = delete;


Here and in CSVDumper: why do you keep deleting copy constructors? 🙃
There must be a reason for it, could you share?

I stopped removing constructors now

IvanNovoselov · 2025-02-04T17:33:24Z

src/common/snippets/include/snippets/op/perf_count.hpp

+ */
+class CSVDumper : public Dumper {
+public:
+    CSVDumper(const std::string &csv_path);


Please pass by value and use std::move to avoid extra copy, like in this comment

aobolensk requested a review from v-Golubev January 10, 2025 15:06

aobolensk requested a review from a team as a code owner January 10, 2025 15:06

github-actions bot added the category: CPU OpenVINO CPU plugin label Jan 10, 2025

aobolensk force-pushed the brgemm-debug-caps branch from e397c92 to 046410a Compare January 10, 2025 15:08

aobolensk requested a review from a team as a code owner January 10, 2025 15:50

aobolensk requested review from tsavina and removed request for a team January 10, 2025 15:50

github-actions bot added the category: docs OpenVINO documentation label Jan 10, 2025

v-Golubev reviewed Jan 13, 2025

View reviewed changes

aobolensk force-pushed the brgemm-debug-caps branch from cfaf273 to 053a7b4 Compare January 13, 2025 13:27

aobolensk requested a review from v-Golubev January 13, 2025 13:37

aobolensk force-pushed the brgemm-debug-caps branch from 053a7b4 to 4aa19d8 Compare January 13, 2025 13:40

v-Golubev reviewed Jan 13, 2025

View reviewed changes

aobolensk force-pushed the brgemm-debug-caps branch from 5ecff19 to f2ab0f0 Compare January 14, 2025 13:51

aobolensk requested a review from a team as a code owner January 14, 2025 13:51

aobolensk force-pushed the brgemm-debug-caps branch 2 times, most recently from 4229be8 to 1fe9e26 Compare January 14, 2025 18:13

aobolensk requested a review from v-Golubev January 15, 2025 08:25

v-Golubev reviewed Jan 15, 2025

View reviewed changes

aobolensk force-pushed the brgemm-debug-caps branch from eb64b0b to c85ad56 Compare January 15, 2025 14:55

v-Golubev reviewed Jan 16, 2025

View reviewed changes

aobolensk force-pushed the brgemm-debug-caps branch from 821ff78 to e4751c5 Compare January 16, 2025 12:58

v-Golubev approved these changes Jan 16, 2025

View reviewed changes

v-Golubev assigned a-sidorova and IvanNovoselov Jan 16, 2025

a-sidorova reviewed Jan 17, 2025

View reviewed changes

src/common/snippets/include/snippets/lowered/pass/brgemm_debug_params.hpp Outdated Show resolved Hide resolved

src/common/snippets/include/snippets/lowered/pass/brgemm_debug_params.hpp Outdated Show resolved Hide resolved

aobolensk force-pushed the brgemm-debug-caps branch 3 times, most recently from a75589b to cb4be59 Compare January 17, 2025 11:04

aobolensk removed request for a team, akuporos and AlexKoff88 January 28, 2025 16:39

aobolensk force-pushed the brgemm-debug-caps branch from 80a8569 to cf39027 Compare January 28, 2025 17:23

IvanNovoselov reviewed Jan 30, 2025

View reviewed changes

aobolensk force-pushed the brgemm-debug-caps branch 2 times, most recently from 355a01c to e2cfa14 Compare January 30, 2025 13:04

IvanNovoselov approved these changes Feb 4, 2025

View reviewed changes

aobolensk added 11 commits February 4, 2025 18:42

[Snippets] Add debug caps for dumping snippets parameters

a24949c

validation

1a57d63

small fixes

81976b4

remove template

ed10b9a

take on extracting dumper

3372ad4

ternary

f4e954e

rename InsertPerfCountVerbose

865e9b7

Address review comments

b2cb397

Resolve simple comments

d0b2a18

move to dtor

e05da86

Address final comments

2c3ee1b

aobolensk force-pushed the brgemm-debug-caps branch from e2cfa14 to 2c3ee1b Compare February 4, 2025 17:43

IvanNovoselov enabled auto-merge February 4, 2025 18:34

IvanNovoselov added this pull request to the merge queue Feb 4, 2025

Merged via the queue into openvinotoolkit:master with commit c27f796 Feb 4, 2025
186 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Snippets] Add debug caps for dumping snippets parameters #28378

[Snippets] Add debug caps for dumping snippets parameters #28378

aobolensk commented Jan 10, 2025

v-Golubev left a comment

aobolensk commented Jan 15, 2025

v-Golubev left a comment

IvanNovoselov Jan 30, 2025

aobolensk Jan 30, 2025

IvanNovoselov Jan 30, 2025

aobolensk Jan 30, 2025

IvanNovoselov Jan 30, 2025

aobolensk Jan 30, 2025

IvanNovoselov Feb 4, 2025

aobolensk Feb 4, 2025

IvanNovoselov Feb 4, 2025

aobolensk Feb 4, 2025

IvanNovoselov Feb 4, 2025

aobolensk Feb 4, 2025

[Snippets] Add debug caps for dumping snippets parameters #28378

[Snippets] Add debug caps for dumping snippets parameters #28378

Conversation

aobolensk commented Jan 10, 2025

Details:

Tickets:

v-Golubev left a comment

Choose a reason for hiding this comment

aobolensk commented Jan 15, 2025

v-Golubev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment